Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

src/plugin/proxy.ts (3)

24-40: Comment says "LRU eviction" but this is actually FIFO.

The cache uses Map insertion order for eviction (oldest-inserted entries are removed first), but a re-used agent (cache hit on line 58) is never moved to the end of the map. True LRU would re-insert on access. With a max of 50 and typical usage of 1–5 proxies this is functionally fine, but the comment is misleading.

---

77-94: Mixed static and dynamic import of undici.

ProxyAgent is statically imported at line 1 (always loaded), while fetch is dynamically imported at line 88. If the intent of the dynamic import is to avoid loading undici when no proxy is configured, the static import on line 1 defeats that purpose. Consider making both imports dynamic, or both static for consistency.

If you want both to be static (simpler):

-import { ProxyAgent } from 'undici';
+import { ProxyAgent, fetch as undiciFetch } from 'undici';

Then in fetchWithProxy, remove the dynamic import:

-  const { fetch: undiciFetch } = await import('undici');

---

88-93: Type incompatibility between global RequestInit and undici's RequestInit is suppressed via @ts-ignore.

The init parameter is typed as the global RequestInit, but undici's fetch expects its own RequestInit (from undici). Properties like signal, body, and headers have subtly different types. The @ts-ignore + as unknown cast hides potential runtime issues. This works today for the simple cases in this PR (plain objects with method, headers, body, signal), but could break if callers pass more exotic RequestInit properties.

Consider typing init as a narrower interface with only the properties actually used, or importing undici's RequestInit and using a union type.

src/plugin/proxy.test.ts (2)

16-22: Agent cache leaks between tests, making test order significant.

Since agentCache is module-private and never cleared between tests, agents created in earlier tests persist. Each test works around this by using unique URLs, but this is fragile — adding a test that reuses a URL from another test will produce confusing failures.

Consider exporting a clearAgentCache() test helper (or using resetModules in beforeEach) to ensure isolation.

---

92-114: Test coverage gaps for proxy utilities.

The fetchWithProxy tests cover the happy paths, but there's no coverage for:

• disposeProxyAgent (exported, untested)
• Unsupported protocol rejection (e.g., socks5://...)
• LRU/FIFO eviction when cache exceeds MAX_AGENT_CACHE_SIZE
• fetchWithProxy with a URL object as input (the .href branch on line 90 of proxy.ts)

These aren't blockers but would strengthen confidence in the module.

src/antigravity/oauth.ts (1)

119-135: Passing undefined as positional placeholder for timeoutMs is a code smell.

fetchWithTimeout now has two trailing optional parameters (timeoutMs, proxyUrl), forcing callers to pass undefined explicitly to reach the proxy parameter (lines 165, 234, 252). Consider using an options object for the optional parameters.

♻️ Suggested refactor

+interface FetchWithTimeoutOptions extends RequestInit {
+  timeoutMs?: number;
+  proxyUrl?: string;
+}

 async function fetchWithTimeout(
   url: string,
-  options: RequestInit,
-  timeoutMs = FETCH_TIMEOUT_MS,
-  proxyUrl?: string,
+  options: FetchWithTimeoutOptions,
 ): Promise<Response> {
+  const { timeoutMs = FETCH_TIMEOUT_MS, proxyUrl, ...fetchOptions } = options;
   const controller = new AbortController();
   const timeout = setTimeout(() => controller.abort(), timeoutMs);
   try {
     if (proxyUrl) {
-      return await fetchWithProxy(url, { ...options, signal: controller.signal }, proxyUrl);
+      return await fetchWithProxy(url, { ...fetchOptions, signal: controller.signal }, proxyUrl);
     }
-    return await fetch(url, { ...options, signal: controller.signal });
+    return await fetch(url, { ...fetchOptions, signal: controller.signal });
   } finally {
     clearTimeout(timeout);
   }
 }

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}